Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

source-postgres: Discover 'citext' columns as strings #2035

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

willdonnelly
Copy link
Member

@willdonnelly willdonnelly commented Oct 9, 2024

Description:

The citext column type is added by the citext extension and stores text with a case-insensitive comparison operator. In my testing it looks like we're capturing these values correctly as strings, but discovery is emitting the catch-all schema which can cause some materializations to do...suboptimal things with those values.

This commit adds "citext" to the discovery type mapping table so it should work correctly, but doesn't add any test cases as it's an extension type (even though the extension is a standard part of the Postgres release and has been for ages so we could probably add the requisite CREATE EXTENSION "citext" statement to the test DB setup script and then treat it like any other built-in type...)


This change is Reviewable

The `citext` column type is added by the `citext` extension and
stores text with a case-insensitive comparison operator. In my
testing it looks like we're capturing these values correctly as
strings, but discovery is emitting the catch-all schema which
can cause some materializations to do...suboptimal things with
those values.

This commit adds `"citext"` to the discovery type mapping table
so it should work correctly, but doesn't add any test cases as
it's an extension type (even though the extension is a standard
part of the Postgres release and has been for ages so we could
probably add the requisite `CREATE EXTENSION "citext"` statement
to the test DB setup script...)
@willdonnelly willdonnelly requested a review from a team October 9, 2024 15:25
@willdonnelly willdonnelly added the change:unplanned Unplanned change, useful for things like doc updates label Oct 9, 2024
Copy link
Member

@williamhbaker williamhbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@willdonnelly willdonnelly merged commit f76e4f8 into main Oct 9, 2024
51 of 53 checks passed
@willdonnelly willdonnelly deleted the wgd/postgres-citext-20241009 branch October 9, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:unplanned Unplanned change, useful for things like doc updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants